Skip to content

Conversation

fredden
Copy link
Member

@fredden fredden commented Jul 14, 2024

Description

The PSR12.Operators.OperatorSpacing sniff has specific logic to avoid an out-of-bounds array offset access. Before now, there were no tests which covered this logic. This commit adds such a test. The sniff already handles this, so there are no changes required there.

This particular omission was detected by infection/infection.

src/Standards/PSR12/Sniffs/Operators/OperatorSpacingSniff.php:104    [M] FalseValue

@@ @@
             }
         } else {
             // Skip partial files.
-            $checkAfter = false;
+            $checkAfter = true;
         }
         if ($checkBefore === true && $tokens[$stackPtr - 1]['code'] !== T_WHITESPACE) {
             $error = 'Expected at least 1 space before "%s"; 0 found';

Suggested changelog entry

No special remarks required in the changelog for this change. Perhaps a generic note about improvements to tests.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
    • This change is only breaking for integrators, not for external standards or end-users.
  • Documentation improvement

PR checklist

  • I have checked there is no other PR open for the same change.
  • I have read the Contribution Guidelines.
  • I grant the project the right to include and distribute the code under the BSD-3-Clause license (and I have the right to grant these rights).
  • I have added tests to cover my changes.
  • I have verified that the code complies with the projects coding standards.
  • [Required for new sniffs] I have added XML documentation for the sniff.

@jrfnl jrfnl added this to the 3.10.x Next milestone Jul 14, 2024
Copy link
Member

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice find and good test addition! Only thing I'm wondering is whether it might be good to mention explicitly in the test comment that this is an "Intentional parse error" ?

The sniff has specific logic to avoid an out-of-bounds array offset access.
Before now, there were no tests which covered this logic. This commit adds such
a test. The sniff already handles this, so there are no changes required there.

This particular omission was detected by infection/infection.

```diff
src/Standards/PSR12/Sniffs/Operators/OperatorSpacingSniff.php:104    [M] FalseValue

@@ @@
             }
         } else {
             // Skip partial files.
-            $checkAfter = false;
+            $checkAfter = true;
         }
         if ($checkBefore === true && $tokens[$stackPtr - 1]['code'] !== T_WHITESPACE) {
             $error = 'Expected at least 1 space before "%s"; 0 found';
```
@fredden fredden force-pushed the mutation-testing/PSR12.Operators.OperatorSpacing branch from 3c9fc66 to addd1da Compare July 14, 2024 19:22
@fredden
Copy link
Member Author

fredden commented Jul 14, 2024

it might be good to mention explicitly in the test comment that this is an "Intentional parse error" ?

I've updated the comment to include this text.

Copy link
Member

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR and the update @fredden! Merging once the build has finished.

@jrfnl jrfnl merged commit 08792e9 into PHPCSStandards:master Jul 14, 2024
@fredden fredden deleted the mutation-testing/PSR12.Operators.OperatorSpacing branch July 14, 2024 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants